Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mx 16271 indexing incoming tokens #325

Open
wants to merge 18 commits into
base: feat/sovereign
Choose a base branch
from

Conversation

axenteoctavian
Copy link

@axenteoctavian axenteoctavian commented Jan 27, 2025

Proposed changes

  • defined a main chain elastic config
  • option to enable/disable this indexation
  • read tokens from the SCR generated by incoming tx processor, if the tokens are not in ES will read them from main chain elastic (another optimization is looking at the token prefix)
  • new run type component IndexTokensHandler

Testing procedure

  • integration tests
  • sovereign local setup

@axenteoctavian axenteoctavian self-assigned this Jan 27, 2025
@axenteoctavian axenteoctavian marked this pull request as ready for review January 28, 2025 14:24
# Conflicts:
#	integrationtests/valuesIndex_test.go
@mariusmihaic mariusmihaic self-requested a review February 3, 2025 09:47
cmd/elasticindexer/config/prefs.toml Outdated Show resolved Hide resolved
data/tokens.go Outdated Show resolved Hide resolved
data/tokens.go Show resolved Hide resolved
factory/runType/runTypeComponentsHandler.go Outdated Show resolved Hide resolved
process/elasticproc/check.go Show resolved Hide resolved
process/elasticproc/elasticProcessor.go Show resolved Hide resolved
@@ -1,4 +1,5 @@
IMAGE_NAME=elastic-container
IMAGE_NAME_2=elastic-container-2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps another name?

scripts/script.sh Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
integrationtests/consts.go Outdated Show resolved Hide resolved
integrationtests/incomingSCR_test.go Outdated Show resolved Hide resolved
scripts/script.sh Outdated Show resolved Hide resolved
factory/runType/sovereignRunTypeComponentsFactory.go Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
process/elasticproc/tokens/sovereignIndexTokensHandler.go Outdated Show resolved Hide resolved
@@ -119,15 +83,30 @@ func (sit *sovereignIndexTokensHandler) getTokensFromScrs(elasticClient elasticp
return newTokens, nil
}

func getTokenCollection(hasPrefix bool, tokenIdentifier string) (bool, string) {
func (sit *sovereignIndexTokensHandler) extractSovereignTokens(tokens []string) []string {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (sit *sovereignIndexTokensHandler) extractSovereignTokens(tokens []string) []string {
func (sit *sovereignIndexTokensHandler) extractNewSovereignTokens(tokens []string) []string {

@@ -23,3 +23,9 @@
username = ""
password = ""
bulk-request-max-size-in-bytes = 4194304 # 4MB

[config.main-chain-elastic-cluster]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you can add a comment here to describe for what is used this section of config

@@ -75,6 +79,9 @@ func (mrtc *managedRunTypeComponents) CheckSubcomponents() error {
if check.IfNil(mrtc.rewardTxData) {
return transactions.ErrNilRewardTxDataHandler
}
if check.IfNil(mrtc.indexTokensHandler) {
return transactions.ErrNilIndexTokensHandler
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you should move transactions.ErrNilIndexTokensHandler in another package, because it is not relayed with the transactions package

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to dataIndexer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you should rename the file in disalbedIndexTokensHandler or similar

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed

return ""
}

func (sit *sovereignIndexTokensHandler) indexNewTokens(responseTokensInfo []data.ResponseTokenInfoDB, buffSlice *data.BufferSlice) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you can rename this function in serializeNewTokens

@@ -33,6 +31,8 @@ type ArgsIndexerFactory struct {
UseKibana bool
ImportDB bool
Sovereign bool
ESDTPrefix string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from where do you read the value of this parameter ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added parameter in config.toml

@@ -13,6 +13,12 @@ import (
"github.com/multiversx/mx-chain-es-indexer-go/process/elasticproc/tokeninfo"
)

// MainChainDatabaseClientHandler defines the actions that sovereign database client handler should do
type MainChainDatabaseClientHandler interface {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this have an IsInterfaceNil func as well?


// NewMainChainElasticClient creates a new sovereign elastic client
func NewMainChainElasticClient(cfg elasticsearch.Config, indexingEnabled bool) (*mainChainElasticClient, error) {
esClient, err := NewElasticClient(cfg)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This again should've been passed as an interface here, not created within this constructor

file: ./Dockerfile
push: true
file: ./Dockerfile-sovereign
push: ${{ github.event_name == 'release' && github.event.action == 'published' }}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we will only use github actions for sovereign and remove the regular ones on this branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants